-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BugFix] Fix metrics error for --num-scheduler-steps > 1 #8234
base: main
Are you sure you want to change the base?
Conversation
👋 Hi! Thank you for contributing to the vLLM project. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can do one of these:
🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. cc @SolitaryThinker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch thank you!
/ready |
Pull request was converted to draft
8b94851
to
3ec84ab
Compare
Hi ! Thanks in advance |
We can merge once the CI is passed (or force merge if the failed tests are flaky) |
@yuleil could you merge in main one more time? tests should be fixed now... |
Can you check if TPOT has also been resolved by this change? |
Fix #8233
Before the fix:
After the fix: